Fix Windows desktop OS build version reporting#1441
Conversation
Use servicing-aware registry values so desktop telemetry reports the current Windows build instead of stale BuildLabEx data after OS updates. Files changed: - lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp - tests/unittests/PalTests.cpp Fixes microsoft#1407 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refactor the Windows OS full-version composition into a testable helper so registry and Rtl build source precedence can be covered without depending on the host machine state. The new PAL tests cover CurrentBuildNumber precedence, CurrentBuild fallback, Rtl build fallback, and missing UBR formatting so future key or fallback regressions are caught before merge. Files changed: - lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp - tests/unittests/PalTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a deterministic PAL test proving that UBR value 0 is still included when the source reports it as present. This keeps zero-value handling separate from the missing-UBR fallback. Files changed: - tests/unittests/PalTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect Windows desktop OS full version reporting by switching from the stale BuildLabEx registry value to servicing-aware build sources (CurrentBuildNumber/CurrentBuild + UBR), with a fallback to RtlGetVersion() when registry values are unavailable. This addresses issue #1407 where serviced builds were reported as their RTM base build.
Changes:
- Replace
BuildLabExusage withCurrentBuildNumber/CurrentBuildandUBR(with fallback toRtlGetVersion()build). - Add helper routines to format and select the OS build string from available sources.
- Add Windows PAL unit tests covering formatting and source-precedence behavior, including UBR-present vs UBR-missing cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/unittests/PalTests.cpp | Adds Windows-only unit tests validating OS full version formatting and precedence/fallback behavior. |
| lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp | Updates Windows desktop OS version detection to use servicing-aware registry values (and fallback to RTL build) instead of BuildLabEx. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@GovM-MSFT from the Windows org is familiar with versioning and his team was the one that reported the issue, so I requested him to review the PR. He could not access the PR here for some reason, but was able to review a mirror I created on my fork with his personal GitHub account and has approved it. |
| } | ||
|
|
||
| std::string getWindowsOsFullVersionFromSources( | ||
| unsigned long majorVersion, |
There was a problem hiding this comment.
One question, and I’m not a Windows expert, so please correct me if I’m missing something: when RtlGetVersion() succeeds, should we use rtlOsvi.dwBuildNumber as the base build and only read UBR from the registry?
This PR currently prefers registry CurrentBuildNumber / CurrentBuild over the RTL build. Those should normally match, but if they ever differ, using RTL as the source of truth for the running OS build seems safer to me.
|
One compatibility note: this changes the shape of the Windows value that flows into Apart from that, changes look good to me - but would let @ThomsonTan approve this, as he knows this part of code better. |
Per Windows-team guidance on issue microsoft#1407: the RtlGetVersion() API is the authoritative source for the running OS build number, and the registry CurrentBuildNumber / CurrentBuild values are only useful for offline reads of another Windows installation. For the running OS the API and registry are always in sync, so prefer the API. UBR is registry-only and is always safe to combine with the kernel-provided build number. Drop the registry CurrentBuildNumber / CurrentBuild precedence chain and the now-unused getCurrentVersionStringValue helper. Format the OS full version directly from rtlOsvi.dwBuildNumber plus the optional UBR. Update PalTests to cover the simplified formatter (UBR present, UBR missing, zero UBR present), replacing the obsolete source-precedence tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Updated this in commit 78e6db4. Confirmed with the Windows team:
So this now uses |
Per review feedback: the Windows team confirmed the registry build
values "should always be in sync" with RtlGetVersion, which means the
registry is a perfectly valid fallback when the API path can't be
reached (e.g., GetProcAddress for RtlGetVersion returning null in a
sandboxed image). Better to emit a correct value from the registry than
an empty ext.os.ver.
Restore the registry build-number lookup as a fallback chain after
RtlGetVersion:
rtlOsvi.dwBuildNumber (preferred, kernel-authoritative)
-> registry CurrentBuildNumber
-> registry CurrentBuild
-> empty osFullVersion
Also fall back to registry CurrentMajorVersionNumber /
CurrentMinorVersionNumber when RtlGetVersion is unavailable, so callers
still get a usable osMajorVersion. UBR is registry-only and is always
safe to combine with whichever build source succeeds.
Update PalTests to cover the new precedence (RTL preferred over
registry, CurrentBuildNumber fallback, CurrentBuild fallback, empty
result when nothing is available) plus the formatter UBR cases.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
BuildLabExto populate the Windows desktopOsBuild/ext.os.verfield.RtlGetVersion().dwBuildNumberfor the running OS build, combined with the registryUBR(Update Build Revision) for serviced builds.Fixes #1407
Field format change (heads-up for downstream consumers)
Before this PR,
DeviceInfo.OsBuild/ext.os.veron Windows desktop could be aBuildLabEx-style string (e.g.26100.1.amd64fre.ge_release.240331-1435). After this PR it is always a numericmajor.minor.build[.ubr]value (e.g.10.0.26100.4061). Dashboards / Kusto queries that parsed the BuildLabEx form will need to be updated; queries that already expected the four-dotted-numeric form will start receiving correct, servicing-aware values.Why this source
Confirmed with the Windows team:
So we prefer
RtlGetVersion()(kernel-authoritative for the running OS) and addUBRfromHKLM\\SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\UBR. The earlier draft that fell back through registryCurrentBuildNumber/CurrentBuildis no longer needed; those values are only useful when reading another Windows installation's hive offline (not our case).Validation
msbuild Solutions\MSTelemetrySDK.sln /t:Tests\UnitTests /p:Configuration=Release /p:Platform=x64 /p:PlatformToolset=v145UnitTests.exe --gtest_filter=PalTests.*→ 10 tests passed, including the three newWindowsOsFullVersion*casesUnitTests.exe(full run) → 463 tests passed